[clang-format] Fix annotating attrs in class/struct #124634
Conversation
|
@llvm/pr-subscribers-clang-format Author: Gedare Bloom (gedare) ChangesAn attribute in a class/struct declaration confuses the parser to treat the identifier following the attribute as a function or variable name. Fixes #124574 Full diff: https://github.com/llvm/llvm-project/pull/124634.diff 2 Files Affected:
diff --git a/clang/lib/Format/TokenAnnotator.cpp b/clang/lib/Format/TokenAnnotator.cpp
index 08fda7a2ffa51a..3b4b0b78e8bf16 100644
--- a/clang/lib/Format/TokenAnnotator.cpp
+++ b/clang/lib/Format/TokenAnnotator.cpp
@@ -2633,12 +2633,20 @@ class AnnotatingParser {
PreviousNotConst->MatchingParen->Previous->isNot(tok::kw_template);
}
- if ((PreviousNotConst->is(tok::r_paren) &&
- PreviousNotConst->is(TT_TypeDeclarationParen)) ||
- PreviousNotConst->is(TT_AttributeRParen)) {
+ if (PreviousNotConst->is(tok::r_paren) &&
+ PreviousNotConst->is(TT_TypeDeclarationParen)) {
return true;
}
+ auto InTypeDecl = [&]() {
+ for (auto Next = Tok.Next; Next; Next = Next->Next)
+ if (Next->isOneOf(TT_ClassLBrace, TT_StructLBrace))
+ return true;
+ return false;
+ };
+ if (PreviousNotConst->is(TT_AttributeRParen) && (!IsCpp || !InTypeDecl()))
+ return true;
+
// If is a preprocess keyword like #define.
if (IsPPKeyword)
return false;
diff --git a/clang/unittests/Format/FormatTest.cpp b/clang/unittests/Format/FormatTest.cpp
index 265461561d2012..6145500a56582a 100644
--- a/clang/unittests/Format/FormatTest.cpp
+++ b/clang/unittests/Format/FormatTest.cpp
@@ -12415,6 +12415,18 @@ TEST_F(FormatTest, UnderstandsAttributes) {
verifyFormat("SomeType s __unused{InitValue};", CustomAttrs);
verifyFormat("SomeType *__capability s(InitValue);", CustomAttrs);
verifyFormat("SomeType *__capability s{InitValue};", CustomAttrs);
+
+ FormatStyle Style = getLLVMStyle(FormatStyle::LK_Cpp);
+ verifyFormat(
+ "template <>\n"
+ "struct __declspec(uuid(\"3895C200-8F26-4F5A-B29D-2B5D72E68F99\"))\n"
+ "IAsyncOperation<IUnknown *> : IAsyncOperation_impl<IUnknown *> {};",
+ Style);
+ verifyFormat(
+ "template <>\n"
+ "class __declspec(uuid(\"3895C200-8F26-4F5A-B29D-2B5D72E68F99\"))\n"
+ "IAsyncOperation<IUnknown *> : IAsyncOperation_impl<IUnknown *> {};",
+ Style);
}
TEST_F(FormatTest, UnderstandsPointerQualifiersInCast) {
|
An attribute in a class/struct declaration confuses the annotater to treat the identifier following the attribute as a function or variable name. Fixes llvm#124574
gedare
force-pushed
the
124574-declspec
branch
from
db6e09d to
5292f11
Compare
gedare
changed the title
HazardyKnusperkeks
left a comment
HazardyKnusperkeks
left a comment
There was a problem hiding this comment.
I'd add an annotating test. (And drop the formatting test.)
|
(Why drop the formatting test? Seems good to have an integration test for this, no?) |
Because the bug was a misannotation. The formatting, based on the annotations, should already be covered. Of course one can always add more tests, and I'm not demanding to remove it, I simply suggest it. The more test cases there are, the longer the duration of building and running the tests for all (clang-format) developers and CIs. |
Yes, that's a good idea, I've done it. I started with the formatting test, but it is unnecessary (although a good, complicated bit of formatting) since the bug was in the annotation. |
| } | ||
|
|
||
| auto InTypeDecl = [&]() { | ||
| for (auto Next = Tok.Next; Next; Next = Next->Next) |
There was a problem hiding this comment.
A more robust and efficient way would be to annotate the class name in UnwrappedLineParser as most of the work has already been done there. I'll submit a patch soon.
|
The improved fix is in #124891. |
5 participants
An attribute in a class/struct declaration confuses the annotator to treat the identifier following the attribute as a function or variable name.
Fixes #124574